-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding shared private link resource for the Azure web pubsub resource #15550
Conversation
}, | ||
|
||
"shared_private_link_resource_types": { | ||
Type: pluginsdk.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a Data Source cannot contain a Set - this needs to be a List
furthermore: what's the intention of this data source? why isn't this part of the WebPubSub data source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This data source is intended to provide a reference of supported sub resource that can be linked by a web pubsub resource, just as the same as the purpose of azurerm_monitor_diagnostic_categories
. I thought it would be more straight-forward to get this information separately since user may would only like to refer to such information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tombuildsstuff may I know if you have any other concerns that I can help to address? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what instances would a user need this information? I searched the repo and I cannot find any other services with such a private link resource data source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first thought was to provide a reference to the user, just in case they don't know what information to put in the propertysubresource_name
. I can remove it if it looks like unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is the entire resource thou? thjats why i am asking what is the use to a user for this resoruce?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I don't quite follow your concerns. I added this data source just in case the user doesn't know what value to put in the subresource_name
when using the resource azurerm_web_pubsub_shared_private_link_resource
the supported subresource varies from different resources, for example, the output of this datasource is:
webpubsub_private_supported_resource = {
"id" = "/subscriptions/733b9054-5ed8-4f10-a444-c32458359d40/resourceGroups/xiaxintest/providers/Microsoft.SignalRService/WebPubSub/acctestWebPubsub"
"shared_private_link_resource_types" = tolist([
{
"description" = "Azure App Service can be used as an upstream"
"subresource_name" = "sites"
},
{
"description" = "Azure Key Vault can be used as credentials store"
"subresource_name" = "vault"
},
])
The user can put either site or vault in the property subresource_name
in below tf block:
resource "azurerm_web_pubsub_shared_private_link_resource" "test" {
name = "acctest-%d"
web_pubsub_id = azurerm_web_pubsub.test.id
subresource_name = "vault"
target_resource_id = azurerm_key_vault.test.id
}
replied inline |
@@ -2,3 +2,4 @@ package signalr | |||
|
|||
//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=WebPubsub -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.SignalRService/WebPubSub/Webpubsub1 | |||
//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=WebPubsubHub -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.SignalRService/WebPubSub/Webpubsub1/hubs/Webpubsubhub1 | |||
//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=WebPubsubSharedPrivateLinkResource -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourcegroups/resGroup1/providers/Microsoft.SignalRService/WebPubSub/Webpubsub1/sharedPrivateLinkResources/resource1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the casing here is wrong, this needs to be:
//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=WebPubsubSharedPrivateLinkResource -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourcegroups/resGroup1/providers/Microsoft.SignalRService/WebPubSub/Webpubsub1/sharedPrivateLinkResources/resource1 | |
//go:generate go run ../../tools/generator-resource-id/main.go -path=./ -name=WebPubsubSharedPrivateLinkResource -id=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/resGroup1/providers/Microsoft.SignalRService/webPubSub/Webpubsub1/sharedPrivateLinkResources/resource1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tombuildsstuff, I will make resourceGroups as camel cased, however, for webPubSub oe, the service is returning the value as WebPubSub and this behavior is the same as its parent WebPubSub resource. Shall we keep it as capital cased?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should be correcting it to the proper case
@katbyte I updated the ID, feel free to let me know if everything looks good to you. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🔝
This functionality has been released in v3.15.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
ACC test: